Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add special region support to @tasks #93

Merged
merged 7 commits into from
Mar 20, 2024
Merged

Add special region support to @tasks #93

merged 7 commits into from
Mar 20, 2024

Conversation

carstenbauer
Copy link
Member

@carstenbauer carstenbauer commented Mar 18, 2024

Addresses #73

  • @one_by_one ("critical")
  • @only_one ("single")

(We don't implement implicit barriers at the end here, or options to turn those on or off. This should go into a new PR targeting #74)

@carstenbauer
Copy link
Member Author

carstenbauer commented Mar 18, 2024

Example:

@tasks for i in 1:10
    @set ntasks = 10

    println(i, ": before")
    @section :critical begin
        println(i, ": one task at a time")
        sleep(1)
    end
    println(i, ": after")
end

@MasonProtter, how do you feel about this syntax, i.e. @section kind begin...end? Alternatively, we could name the "macro" @region but I think I like @section more.

@carstenbauer
Copy link
Member Author

carstenbauer commented Mar 18, 2024

julia> @tasks for i in 1:10
           @set ntasks = 10
       
           println(i, ": before")
           @section :single begin
               println(i, ": only printed by a single task")
               sleep(1)
           end
           println(i, ": after")
       end
4: before
10: before
9: before
1: before
9: after
1: after
5: before
3: before
7: before
8: before
7: after
8: after
3: after
4: only printed by a single task
6: before
10: after
6: after
2: before
2: after
5: after
4: after

@MasonProtter
Copy link
Member

Honestly, as someone who has never used OpenMP, I don't really find any of these names informative or obvious.

I think "region" is a bit better than section for me, but I find "single" and "critical" are both quite bad. On the other hand, there's advantages to using standardized terminology from another piece of software, and I don't anticipate myself really needing these things, so I'm not sure I have particularly strong feelings.

@carstenbauer
Copy link
Member Author

carstenbauer commented Mar 18, 2024

Honestly, as someone who has never used OpenMP, I don't really find any of these names informative or obvious.

Note that critical section (or region) isn't an OpenMP term but just standard terminology. As for "single", isn't that as obvious as it gets? The section will be run by a single task only.

What would you suggest instead?

@MasonProtter
Copy link
Member

Yeah if it's widespread terminology, I'm hesitant to go against it, but my reflex would be to try and give it a more descriptive name like e.g. @one_at_a_time begin ... end and @only_one_task begin ... end. This is verbose, but to be fair, it's also fewer characters than @section :critical begin ... end and @section :single begin ... end.

I don't feel particularly strongly about this, these are just my thoughts.

@carstenbauer
Copy link
Member Author

carstenbauer commented Mar 19, 2024

I like more descriptive names, so that's a good point (irrespective of standard terminology). Brainstorming:

@one_task_only and @one_task_at_a_time

@only_one and @one_by_one

@single_task and @each_task_alone

@single and @each_alone

I must say that @only_one and @one_by_one is growing on me because it's more descriptive but also quite short.

@carstenbauer carstenbauer changed the title Add @section support to @tasks Add special region support to @tasks Mar 19, 2024
@carstenbauer
Copy link
Member Author

carstenbauer commented Mar 19, 2024

With the new (tentative) macro names:

julia> using OhMyThreads: @tasks

julia> @tasks for i in 1:10
           @set ntasks = 10
       
           println(i, ": before")
       
           @one_by_one begin
               println(i, ": printed by all tasks but one after another")
               sleep(0.5)
           end
       
           @only_one begin
               println(i, ": only printed by a single task")
               sleep(1)
           end
       
           println(i, ": after")
       end
1: before
10: before
4: before
2: before
9: before
5: before
3: before
6: before
8: before
7: before
1: printed by all tasks but one after another
1: only printed by a single task
10: printed by all tasks but one after another
10: after
5: printed by all tasks but one after another
1: after
5: after
3: printed by all tasks but one after another
3: after
4: printed by all tasks but one after another
4: after
8: printed by all tasks but one after another
8: after
2: printed by all tasks but one after another
2: after
6: printed by all tasks but one after another
6: after
7: printed by all tasks but one after another
7: after
9: printed by all tasks but one after another
9: after

Copy link
Member

@MasonProtter MasonProtter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice stuff. I like the names, and I like the implementation and the way you tested it. Can't spot any problems.

@carstenbauer carstenbauer mentioned this pull request Mar 20, 2024
@carstenbauer carstenbauer merged commit 761063f into master Mar 20, 2024
10 checks passed
@carstenbauer carstenbauer deleted the cb/sections branch March 20, 2024 09:36
@carstenbauer carstenbauer restored the cb/sections branch June 28, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants